fix(client): log exceptions during client finalization in __del__ - #3525
fix(client): log exceptions during client finalization in __del__#3525ZachDreamZ wants to merge 1 commit into
Conversation
ting-hong-shieh
left a comment
ting-hong-shieh
left a comment
There was a problem hiding this comment.
Requesting one change on exact head c03c0a0b and its clean synthetic merge 63b7d36c with current main d9029e3a:
base e67afa88: []
head c03c0a0b: [AttributeError, AttributeError]
current-main merge: [AttributeError, AttributeError]
head with guarded log: []
The same fixture forces sync close() failure and async scheduling failure, clears the module logger to model finalization, and captures sys.unraisablehook. Ruff lint/format and git diff --check pass, so the remaining issue is the destructor's no-throw behavior rather than formatting. No API call, credentials, or external runtime service was used.
| self.close() | ||
| except Exception: | ||
| pass | ||
| log.debug("Failed to auto-close client during finalization", exc_info=True) |
There was a problem hiding this comment.
__del__ can run during interpreter shutdown after module globals have been cleared or set to None. This new diagnostic call is not guarded, so a close failure can now escape the destructor as an unraisable AttributeError when log is unavailable.
Using the same broken sync/async wrappers with base_client.log = None, base e67afa88 records [], while this head records two "'NoneType' object has no attribute 'debug'" events through sys.unraisablehook; the result is unchanged on the clean merge with current main d9029e3a.
Please keep the diagnostic, but guard the logging call itself and add regression coverage for both wrappers. A nested try around log.debug(...) restored [] locally and passed Ruff lint/format. Python documents the shutdown/global-state constraint for __del__.
2 participants
Summary
Addresses #3428 by adding debug logging for exceptions raised during client finalization in del.
Description
Previously, exceptions raised in \SyncHttpxClientWrapper.del\ and \AsyncHttpxClientWrapper.del\ were silently swallowed via \pass. Logging them with \log.debug(..., exc_info=True)\ provides diagnostic visibility during debugging without changing runtime error flow.
Authored by Carl Andrie Ellepure (@ZachDreamZ).